Skip to content

Fix/46642/groups values in pdf - #95

Merged
Rom1-B merged 14 commits into
11.0/bugfixesfrom
fix/46642/groups-values-in-pdf
Oct 28, 2026
Merged

Fix/46642/groups values in pdf#95
Rom1-B merged 14 commits into
11.0/bugfixesfrom
fix/46642/groups-values-in-pdf

Conversation

@jdurand-teclib

@jdurand-teclib jdurand-teclib commented Oct 1, 2026

Copy link
Copy Markdown
  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

  • It fixes !46642
  • Here is a brief description of what this PR does
    • Fix groups values display in PDF (groups_id & groups_id_tech) for every concerned assets/items. groups values were treated as single id where it is actually a list of ids.
    • Applied striptag to groups names
    • Added unit tests

Screenshots

Testing asset:
image

Before:
image

After:
image

@jdurand-teclib
jdurand-teclib force-pushed the fix/46642/groups-values-in-pdf branch from 465949d to 23773b8 Compare October 1, 2026 13:19
@jdurand-teclib jdurand-teclib self-assigned this Oct 5, 2026
Mary-Clb
Mary-Clb approved these changes Oct 6, 2026
View reviewed changes

@Mary-Clb Mary-Clb left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looks like the same problem as #63 (group fields loaded as arrays of IDs for assignable assets). Yours is more complete (more asset classes covered), so I'll close #63 in favor of this one.

@jdurand-teclib
jdurand-teclib changed the base branch from main to 11.0/bugfixes October 7, 2026 13:15
@jdurand-teclib

jdurand-teclib commented Oct 7, 2026

Copy link
Copy Markdown
Author

I gave the patch to the customers, I'm waiting for their feedback before merging

RomainLvr
RomainLvr requested changes Oct 12, 2026
View reviewed changes
Comment thread inc/domain_item.class.php Outdated
@Rom1-B
Rom1-B self-requested a review October 19, 2026 12:10
Rom1-B
Rom1-B reviewed Oct 19, 2026
View reviewed changes

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same root cause remains in item_softwarelicense.class.php:208: pdfForLicenseByComputer() crashes with Unknown column 'glpi_computers.groups_id', and item_softwareversion.class.php:210 always shows an empty Group column. Can you fix them here or in a follow-up PR?

@Mary-Clb #63 is still open; can you close it in favor of this one as planned?

Comment thread inc/common.class.php Outdated
Lainow
Lainow reviewed Oct 22, 2026
View reviewed changes

@Lainow Lainow left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase

Rom1-B
Rom1-B requested changes Oct 26, 2026
View reviewed changes
Comment thread tests/Units/GroupsDisplayTest.php
Comment thread tests/Units/ItemSoftwareVersionTest.php
Comment thread tests/bootstrap.php Outdated
Comment thread CHANGELOG.md Outdated
Rom1-B
Rom1-B reviewed Oct 26, 2026
View reviewed changes
Comment thread .gitignore Outdated
@jdurand-teclib @Rom1-B
Update tests/bootstrap.php
794cbba 
Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com>
jdurand-teclib and others added 2 commits October 26, 2026 16:04
@jdurand-teclib @Rom1-B
Update .gitignore
2acdc8e 
Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com>
@jdurand-teclib
chores(46642): license header update, unit tests files moved, changel…
49e2a0c 
…og updated
@Herafia

Herafia commented Oct 26, 2026

Copy link
Copy Markdown

@jdurand-teclib
Can you check to see if the same problem exists? (groups_id stored as an array), which is still present elsewhere.

  • inc/group.class.php (pdfItems): getField(‘groups_id’) returns an array, which is passed as-is to getFromDB(). The “Group / User” column is therefore incorrect or empty.
  • inc/user.class.php (lines 288 to 320, approximately): The search is performed on the groups_id column, and isField() returns false for assignable types. The items associated with the user’s groups no longer appear in the export. This does not cause a crash, but data is missing

Herafia
Herafia requested changes Oct 26, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
@jdurand-teclib

jdurand-teclib commented Oct 26, 2026

Copy link
Copy Markdown
Author

@jdurand-teclib Can you check to see if the same problem exists? (groups_id stored as an array), which is still present elsewhere.

* `inc/group.class.php` (`pdfItems`): `getField(‘groups_id’)` returns an array, which is passed as-is to `getFromDB()`. The “Group / User” column is therefore incorrect or empty.

* `inc/user.class.php` (lines 288 to 320, approximately): The search is performed on the `groups_id` column, and `isField()` returns `false` for assignable types. The items associated with the user’s groups no longer appear in the export. This does not cause a crash, but data is missing

The issue was indeed real. It is now fixed with a Unit test to cover the changes

@jdurand-teclib
jdurand-teclib requested review from Herafia, MyvTsv and Rom1-B and removed request for Lainow, MyvTsv and RomainLvr October 26, 2026 15:44
@Rom1-B
Rom1-B merged commit fe04ec3 into 11.0/bugfixes Oct 28, 2026
7 checks passed
@Rom1-B
Rom1-B deleted the fix/46642/groups-values-in-pdf branch October 28, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lainow Lainow Lainow left review comments
@Rom1-B Rom1-B Rom1-B approved these changes
@Herafia Herafia Herafia approved these changes
@MyvTsv MyvTsv MyvTsv approved these changes
@Mary-Clb Mary-Clb Mary-Clb approved these changes
@RomainLvr RomainLvr RomainLvr approved these changes

Assignees

@jdurand-teclib jdurand-teclib

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jdurand-teclib @Herafia @Rom1-B @MyvTsv @Mary-Clb @RomainLvr @Lainow